76dae5d cmake: Replace recursive globbing with explicit globbing in folders (Hennadii Stepanov)
88d9092 cmake: Create subdirectories in build tree in advance (Hennadii Stepanov)

Pull request description:

  While reviewing #32697, I noticed that symlink creation fails when the target subdirectory does not exist. In such cases, `file(CREATE_LINK ... COPY_ON_ERROR SYMBOLIC)` falls back to copying, which implicitly creates the required path. As a result, a single file is copied instead of symlinked for subdirectories in `test/functional`.

  This PR ensures that necessary subdirectories are created in advance, so that subsequent symlink creation does not fail due to missing paths.

  For example:
  - on the master branch:
  ```
  $ cmake -B build
  $ ls -l build/test/functional/mocks/
  total 8
  -rwxrwxr-x 1 hebasto hebasto 2683 Jul  3 14:11 invalid_signer.py
  lrwxrwxrwx 1 hebasto hebasto   64 Jul  3 14:11 multi_signers.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/multi_signers.py
  lrwxrwxrwx 1 hebasto hebasto   60 Jul  3 14:11 no_signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/no_signer.py
  lrwxrwxrwx 1 hebasto hebasto   57 Jul  3 14:11 signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/signer.py
  ```
  - with this PR:
  ```
  $ cmake -B build
  $ ls -l build/test/functional/mocks/
  total 4
  lrwxrwxrwx 1 hebasto hebasto 65 Jul  3 13:51 invalid_signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/invalid_signer.py
  lrwxrwxrwx 1 hebasto hebasto 64 Jul  3 13:51 multi_signers.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/multi_signers.py
  lrwxrwxrwx 1 hebasto hebasto 60 Jul  3 13:51 no_signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/no_signer.py
  lrwxrwxrwx 1 hebasto hebasto 57 Jul  3 13:51 signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/signer.py
  ```

ACKs for top commit:
  m3dwards:
    ACK 76dae5d
  sedited:
    ACK 76dae5d
  janb84:
    re ACK 76dae5d

Tree-SHA512: a720a68752c72390b9452b192b06d09e41cac1080d32cfe3c2caabb65949626771e0709e7193f69677bd24a3c747e368c2323d9c857d4aa97e1890cc463850ed

…h unit tests

8c03318 consensus/doc: explain `GetValueOut()` precondition (Lőrinc)
82ef92c consensus/doc: explain unreachable `bad-txns-fee-outofrange` check (Lőrinc)
232a2bc consensus/test: add out-of-range output unit tests for `CTransaction::GetValueOut` (Lőrinc)
aa87aae consensus/test: add `MoneyRange` unit tests for `CheckTxInputs` (Lőrinc)

Pull request description:

  ### Problem
  Coverage reports indicate that a few consensus related validations aren't exercised in unit-, and some not even in the functional-tests:
  Inspired by the coverage reports:
  * ["bad-txns-premature-spend-of-coinbase"](https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/consensus/tx_verify.cpp.gcov.html#L180): Only covered in functional tests
  * ["bad-txns-inputvalues-outofrange"](https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/consensus/tx_verify.cpp.gcov.html#L187): Unreachable in functional tests [1], uncovered in unit tests
  * ["bad-txns-in-belowout"](https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/consensus/tx_verify.cpp.gcov.html#L193): Only covered in functional tests
  * ["GetValueOut: value out of range"](https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/primitives/transaction.cpp.gcov.html#L103) and [total coverage report](https://maflcko.github.io/b-c-cov/total.coverage/src/primitives/transaction.cpp.gcov.html#L103)

  Replacing them with explicit throws still passes all unit (and sometimes even functional) tests, confirming those branches are not being exercised, see: l0rinc#112

  ### Fixes

  Add minimal unit test coverage for `Consensus::CheckTxInputs` invalid outcomes for `bad-txns-premature-spend-of-coinbase`, `bad-txns-inputvalues-outofrange`, `bad-txns-in-belowout`.
  Add a unit test covering `CTransaction::GetValueOut()` throwing for out of range values.
  After the prerequisits are tested, document why `bad-txns-fee-outofrange` is unreachable - while keeping the check in place because it is consensus-critical code.

ACKs for top commit:
  maflcko:
     lgtm ACK 8c03318 🍴
  darosior:
    utACK 8c03318
  sedited:
    ACK 8c03318

Tree-SHA512: 91c65dda99b42d12de99c58b02df0f861203f97d268329a3ecce79bd681fcaf847f508c1d9f2256b2b92a953a94d868cbae647f378def92484681d771722ea27

…InvalidateBlock

- there is no functional difference between BLOCK_FAILED_VALID and BLOCK_FAILED_CHILD
and it's unnecessary code complexity to correctly categorise them.

fa16b27 test: Check that interrupt results in EXIT_SUCCESS (MarcoFalke)
fab7c7f test: Split large init_stress_test into two smaller functions (MarcoFalke)

Pull request description:

  This is a test for #34224. The test can be tested by reverting that pull request and observing the test failure.

  Also, includes a small test cleanup/refactor.

ACKs for top commit:
  janb84:
    ACK fa16b27
  sedited:
    ACK fa16b27

Tree-SHA512: bb6894316fd4f78b3c0cb299cc93abf7f3f794e0507bf7deda7d86f8f45d60299ec0f027c41a6f909c197a1e5fba44fe269ce4165450b89738b82d8ddf196b80

…asts (take 2)

fa68013 refactor: [rpc] Remove confusing and brittle integral casts (take 2) (MarcoFalke)

Pull request description:

  Second take for cases which I seem to have missed in commit 94ddc2d.

  When constructing an UniValue from integral values, historically (long ago), in some cases casts where needed. With the current UniValue constructor, only very few are actually needed.

  Keeping the unused casts around is:

  * confusing, because code readers do not understand why they are needed
  * brittle, because some may copy them into new places, where they will lead to hard-to-find logic bugs, such as the ones fixed in pull #34112

  So fix all issues by removing them, except for a few cases, where casting was required:
  * `static_cast<bool>(fCoinBase)`, because `bool{fCoinBase}` only works on modern compilers.

  This hardening refactor does not fix any bugs and does not change any behavior.

ACKs for top commit:
  Sjors:
    ACK fa68013
  sedited:
    ACK fa68013

Tree-SHA512: 77f03f496ea2a42987720cb4a36eb4e7a0d5b512ed7f029e41dd54c04fb4c1680f7cc13514acbc9f1bae991be4de3cf31c5a0d27c016a030f5749d132603f71e

…llyCopyableMove=false

fa88ac3 doc: Clarify why performance-move-const-arg.CheckTriviallyCopyableMove=false (MarcoFalke)

Pull request description:

  Without this doc, there is a risk that the setting will be turned off, see #34514.

  The reason to disable it is to catch logic bugs, even on trivially copyable types:

  ```cpp
  #include <utility>
  void Eat(int&& food) { food = 0; };
  int main() {
    int food{2};
    Eat(std::move(food));
    Eat(std::move(food));  // This should err
  }
  ```

ACKs for top commit:
  l0rinc:
    ACK fa88ac3
  hebasto:
    ACK fa88ac3.
  sedited:
    ACK fa88ac3

Tree-SHA512: d1bda846a10190a2936084a06bd87418c6a3e4ababc298e4beb9bc9e1190bff430cbe973475d634eda5ef7863571c89bfa4b78ff63fcbd9ac10c42fd9d5fa23a

fe0b151 test: add a test for txgraph staging (Hao Xu)
ef253a9 test: add block builder tests for txgraph (Hao Xu)
4a1ac31 test: add a chunk test for txgraph (Hao Xu)

Pull request description:

  Add tests for cluster chunks, including:
      - txgraph_chunk_chain test: test chunk implementation for a simple chain style graph .
      - txgraph_staging test: test the staging feature for a basic graph.

ACKs for top commit:
  instagibbs:
    reACK fe0b151
  sipa:
    reACK fe0b151

Tree-SHA512: 01010a3b4e4163849df2912d1393be74d26eb199d0544cfbef58a498aca5153463a118f55a2f1cad2995552b74210031e659de8df6b88cbcffdffd2a1b464990

fa90277 ci: Use ubuntu-slim for [meta] runners (MarcoFalke)
fa9627a ci: Rely on cmake --preset toolchain file (MarcoFalke)
fa3f89a ci: Add check_manifests to ci-windows.py (MarcoFalke)
1111079 ci: Add run_tests step to ci-windows.py (MarcoFalke)
fa56168 ci: [refactor] Add .github/ci-windows.py prepare_tests step (MarcoFalke)
fa3e607 ci: Print verbose Windows CI build failure (MarcoFalke)
4444808 ci: [refactor] Add .github/ci-windows.py build step (MarcoFalke)
fabdd4e ci: Refactor Windows CI into script (MarcoFalke)

Pull request description:

  Just like all the other CI configs, the Windows one should print a single and readable build failure at the end.

  Also, includes a bunch of Windows CI refactors.

ACKs for top commit:
  m3dwards:
    ACK fa90277
  hebasto:
    ACK fa90277.
  willcl-ark:
    utACK fa90277

Tree-SHA512: 00443289e3d8b6d60d1347934d9d4b16098e8c36b6325467e5804b1869714201c4f7e932da3be44392c73e4713a1f52cd8af481894d36c6a281ba7238d43434e

Instead of returning a TxGraph::Ref from TxGraph::AddTransaction(),
pass in a TxGraph::Ref& which is updated to refer to the new transaction
in that graph.

This cleans up the usage somewhat, avoiding the need for dummy Refs in
CTxMemPoolEntry constructor calls, but the motivation is that a future
commit will allow a callback to passed to MakeTxGraph to define a
fallback order on the transaction objects. This does not work when a
Ref is created separately from the CTxMemPoolEntry it ends up living in,
as passing the newly-created Ref to the callback would be UB before it's
emplaced in its final CTxMemPoolEntry.

This makes TxGraphImpl::Compact() invoke Cluster::Updated() on all
affected clusters, in case they have internal GraphIndex values stored
that may have become outdated with the renumbering of GraphIndex values
that Compact() caused.

No such GraphIndex values are currently stored, but this will change in
a future commit.

Whenever a TxGraph::Ref is destroyed, if it by then still appears inside
main-level clusters, wipe the chunk index entries for those clusters, to
prevent having lingering indexes for transactions without Ref.

This is preparation for enabling a callback being passed to MakeTxGraph
to define a fallback order on objects. Once the Ref for a transaction is
gone, it is not possible to invoke the callback anymore. To prevent the
index becoming inconsistent, we need to immediately get rid of the index
entries when the Ref disappears.

This is not a problem, because such destructions necessarily will
trigger a relinearization of the cluster (assuming there are
transactions in it left) before becoming acceptable again, and the chunk
ordering is not observable (through CompareMainOrder, or through the
BlockBuilder interface) until that point. However, the index itself
needs to remain consistent in the mean time, even if not meaningful.

This changes the order of transactions within a chunk to be:
1. Topology (parents before children)
2. Individual transaction feerate (high to low)
3. Individual transaction weight (small to large)
4. Random tiebreak (will be changed in a future commit)

To do so, use a heap of topology-ready transactions within
GetLinearization(), sorted by (2), (3), and (4).

This is analogous to the order of chunks within a cluster, which is
unchanged:
1. Topology (chunks after chunks they depend on)
2. Chunk feerate (high to low)
3. Chunk weight (small to large)
4. Random tiebreak (will be changed in a future commit)

…eature)

This makes TxGraph track the equal-feerate-prefix size of all chunks in
all clusters in the main graph, and uses it to sort chunks coming from
distinct clusters.

The order of chunks across clusters becomes:
1. Feerate (high to low)
2. Equal-feerate-prefix (small to large)
3. Cluster sequence number (old to new); this will be changed later.

The equal-feerate-prefix size of a chunk C is defined as the sum
of the weights of all chunks in the same cluster as C, with the same
feerate as C, up to and including C itself, in linearization order (but
excluding such chunks that appear after C).

This is an approximation of sorting chunks from small to large across
clusters, while remaining consistent with intra-cluster linearization
order.

This allows passing in a fallback order comparator to Linearize(), which
is used as final tiebreak when deciding the order of chunks and
transactions within a chunk, rather than a random tiebreak.

The order of transactions within a chunk becomes:
1. Topology (parents before children)
2. Individual transaction feerate (high to low)
3. Weight (small to large)
4. Fallback (low to high fallback order)

The order of chunks within a cluster becomes:
1. Topology (chunks after their dependencies)
2. Feerate (high to low)
3. Weight (small to large)
4. Max-fallback (chunk with lowest maximum-fallback-tx first)

For now, txgraph passes a naive comparator to Linearize(), which makes
the cluster order deterministic when treating the input transactions as
identified by the DepGraphIndex. However, since DepGraphIndexes are the
result of possibly-randomized operations inside txgraph, this doesn't
actually make txgraph's per-cluster ordering deterministic. That will be
changed in a later commit, by using a txid-based fallback instead.

This is a small change to the txgraph fuzz test to make it used objects
derived from TxGraph::Ref (SimTxObject) rather than TxGraph::Ref
directly. This matches how the mempool uses CTxMemPoolEntry, which
derives from TxGraph::Ref.

This is preparation for a future commit which will introduce simulated
txids to the transactions in this fuzz test, to be used as fallback
order.

This adds an std::function<strong_ordering(Ref&,Ref&)> argument to the
MakeTxGraph function, which can be used by the caller (e.g., mempool
code) to provide a fallback order to TxGraph.

This is just preparation; TxGraph does not yet use this fallback order
for anything.

Add glue to make TxGraph use the fallback order provided to it, in the
fallback comparator it provides to the cluster linearization code.

The order of chunks within a cluster becomes:
1. Topology (chunks after their dependencies)
2. Feerate (high to low)
3. Weight (small to large)
4. Max-txid (chunk with lowest maximum-txid first)

The order of transactions within a chunk becomes:
1. Topology (parents before children)
2. Individual transaction feerate (high to low)
3. Weight (small to large)
4. Txid (low to high txid)

This makes optimal cluster linearization, both the order of chunks
within a chunk, and the order of transactions within those chunks,
completely deterministic.

This makes TxGraph also use the fallback order to decide the order of
chunks from distinct clusters.

The order of chunks across clusters becomes:
1. Feerate (high to low)
2. Equal-feerate-chunk-prefix (small to large)
3. Max-txid (chunk with lowest maximum-txid first)

This makes the full TxGraph ordering fully deterministic as long as all
clusters in it are optimally linearized.

This is almost a refactor. The only change is putting the
bitcoind.manifest into a different folder.

…indows-cross.py

This is mostly a refactor, except for putting the temp dirs into
Path.cwd(), which makes running this locally easier.

Note, the use of process_cpu_count() is intentional. It was only added
in Python 3.13, according to
https://docs.python.org/3/library/os.html#os.process_cpu_count .

However, Python 3.13 is also the minimum required version on Windows,
according to
#29897 (comment)
to avoid intermittent test failures.

This uses the workspace instead of the runner.temp, so that it is easier
to reproduce the CI locally.

…tion' less error-prone

4c0d4f6 refactor: interfaces, make 'createTransaction' less error-prone (furszy)
e2c3ec9 refactor: move CreatedTransactionResult to types.h (furszy)
4537217 gui: remove AmountWithFeeExceedsBalance error special case (furszy)

Pull request description:

  Bundle all function's outputs inside the `util::Result` returned object.

  Removals:
  - The input-output 'change_pos' ref arg from `createTransaction`, which has been a source of bugs in the past.
  - The 'fee' ref arg from `createTransaction`, which is currently only set when the transaction creation process succeeds.
  - The no longer needed `AmountWithFeeExceedsBalance` error (more info about its re-introduction at [#25269](#25269) and [#34299](#34299).

  Additionally, this PR moves the `CreatedTransactionResult` struct into its own file. This change is made to avoid further expanding the GUI dependencies on `wallet.h`. Structurally, the GUI should only access the model/interfaces and never the wallet directly.

ACKs for top commit:
  stratospher:
    ACK 4c0d4f6.
  hebasto:
    ACK 4c0d4f6.

Tree-SHA512: 4fc61f08ca2e66e46001defb3a2e852265713e75006c98f0c465bd48afe42e7b0d626d28d578741906fdd26e907d6919f06dc640c55c44efc3dfa766fdbf38a4

This ensures clang-format can properly restore empty lines between
header groups that were previously stripped by fix_includes.py.

Move the operator<< overloads used by BOOST_CHECK_* out of the
unit test machinery test/setup_common, into test/util/common.h.

And replace the individual per-type ToString() overloads with
a single concept-constrained template that covers any type
exposing a ToString() method. This is important to not add
uint256.h and transaction_identifier.h dependencies to the
shared test/util/common.h file.

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>

Avoid providing the entire unit test framework dependency to tests that only
require access to the HasReason utility class.

E.g. reverselock_tests.cpp, sync_tests.cpp, util_check_tests.cpp, util_string_tests.cpp,
and script_parse_tests.cpp only require access to HasReason and nothing else.

Submit tasks to a non-started, interrupted, or stopped
pool and verify the proper error is always returned.

Useful for debugging issues.

Co-authored-by: Matias Furszyfer <matiasfurszyfer@protonmail.com>

The standard and fuzz matrix jobs share the same github.job value
(windows-native-dll), so both try to save the vcpkg tools cache with the
same key.

Since the tools are identical across build types, let them share a
single cache entry by restricting the save to the standard job only.

… matrix jobs

17a079c ci: fix vcpkg tools cache key collision between windows matrix jobs (will)

Pull request description:

  The standard and fuzz matrix jobs share the same github.job value
  (windows-native-dll), so both try to save the vcpkg tools cache with the
  same key.

  Since the tools are identical across build types, let them share a
  single cache entry by restricting the save to the standard job only.

ACKs for top commit:
  maflcko:
    lgtm ACK 17a079c
  hebasto:
    ACK 17a079c, this should fix issues like #34559 (comment).

Tree-SHA512: 2e83846bfc88947b4bc321baa23563e0c093cd4f55f11f8105c2ecf867b78028aa71aa4780f928103b7a9f1f2e3cf72dbb072f05e7925bc1d00069234acf23c9

0a8d303 test: fix test_limit_enforcement_package (Greg Sanders)

Pull request description:

  The current test has a couple issues:

  1) the parent_tx_good is regenerating the exact same transaction that is already in the cluster, so it's resulting in no replacements on submission
  2) once fixed, the additional fee needs to be allocated to the parent transaction in the package, not the child. If the RBF fees are allocated to the child, this triggers the package RBF logic, which requires no in-mempool ancestors to be present.

  Fix the bug and add a few assertions to protect against regressions.

ACKs for top commit:
  bensig:
    ACK 0a8d303
  achow101:
    ACK 0a8d303
  sipa:
    ACK 0a8d303

Tree-SHA512: 0ba184d82edc5a502e9119a6876e80c4564c876fa51ee39293d47bd30c18bf3ded50fbd2f6f2a3394784fad05d8f6370a90682068b30358b077280abd2477252

Appveyor is not longer used however the test still requires to check for
permissions including 666 as otherwise the tests fail on Windows

… dependency cleanup

408d5b1 test: include response body in non-JSON HTTP error msg (Matthew Zipkin)
9dc653b test: threadpool, add coverage for all Submit() errors (furszy)
ce2a984 test: cleanup, use HasReason in threadpool_tests.cpp (l0rinc)
d9c6769 test: refactor, decouple HasReason from test framework machinery (furszy)
dbbb780 test: move and simplify BOOST_CHECK ostream helpers (Hodlinator)
3b7cbca test: ensure Stop() thread helps drain the queue (seduless)
ca101a2 test: coverage for queued tasks completion after interrupt (furszy)
bf2c607 threadpool: active-wait during shutdown (furszy)
e88d274 test: add threadpool Start-Stop race coverage (furszy)
8cd4a43 threadpool: guard against Start-Stop race (furszy)
9ff1e82 test: cleanup, block threads via semaphore instead of shared_future (l0rinc)

Pull request description:

  A few follow-ups to #33689, includes:

  1) `ThreadPool` active-wait during shutdown:
  Instead of just waiting for workers to finish processing tasks, `Stop()` now helps them actively.
  This speeds up the JSON-RPC and REST server shutdown, resulting in a faster node shutdown when many requests remain unhandled. This wasn't included in the original PR due to the behavior change this introduces.

  2) Decouple `HasReason` from the unit test framework machinery
  This avoids providing the entire unit test framework dependency to low-level tests that only require access to the `HasReason` utility class. Examples are: `reverselock_tests.cpp`, `sync_tests.cpp`, `util_check_tests.cpp`, `util_string_tests.cpp`, `script_parse_tests.cpp` and `threadpool_tests.cpp`. These tests no longer gain access to unnecessary components like the chainstate, node context, caches, etc. It includes l0rinc's `threadpool_tests.cpp` `HasReason` changes.

  3) Include response body in non-JSON HTTP error messages
  Straight from pinheadmz [comment](#33689 (comment)), it makes debugging CI issues easier.

ACKs for top commit:
  maflcko:
    review ACK 408d5b1 🕗
  achow101:
    ACK 408d5b1
  hodlinator:
    re-ACK 408d5b1

Tree-SHA512: 57aa0ef96886f32bf95a0bd7f87c878d31c9df9e34cb96de615eee703ce0824b5cfdf8f5c9cd19a3594559994295b5810c38c94f5efd6291cbbd83a95473357a

```
src/common/netif.cpp:137:51: error: comparison of integers of different signs: 'int64_t' (aka 'long') and 'unsigned long' [-Werror,-Wsign-compare]
  137 |         for (nlmsghdr* hdr = (nlmsghdr*)response; NLMSG_OK(hdr, recv_result); hdr = NLMSG_NEXT(hdr, recv_result)) {
      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/netlink/netlink.h:220:31: note: expanded from macro 'NLMSG_OK'
  220 | #define NLMSG_OK(_hdr, _len)            NL_ITEM_OK(_hdr, _len, NLMSG_HDRLEN, _NLMSG_LEN)
      |                                         ^                ~~~~  ~~~~~~~~~~~~
/usr/include/netlink/netlink.h:203:10: note: expanded from macro 'NL_ITEM_OK'
  203 |         ((_len) >= _hlen && _LEN_M(_ptr) >= _hlen && _LEN_M(_ptr) <= (_len))
      |           ~~~~  ^  ~~~~~
1 error generated.
```

Happens on FreeBSD 15.0, with the default compiler (Clang 19).

On FreeBSD 14, `/usr/include/netlink/netlink.h` contains:
```
 #define NLMSG_HDRLEN                    ((int)sizeof(struct nlmsghdr))
```

On FreeBSD 15, `/usr/include/netlink/netlink.h` contains:
```
 #define NLMSG_HDRLEN                    (sizeof(struct nlmsghdr))
```

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

8834e4e test: remove appveyor reference in comment (Max Edwards)

Pull request description:

  Appveyor is not longer used however the test still requires to check for permissions including 666 as otherwise the tests fail on Windows

  Fixes: #32576

ACKs for top commit:
  maflcko:
    lgtm ACK 8834e4e
  hebasto:
    ACK 8834e4e.

Tree-SHA512: 655b44e52da5e0c6c11c79bb4f92c701c6e0e66dce8d7791ccf1d64e4561fe4d1d5f37c1317bead89c88e4d7208a278925168b419482a6be17abf93d0ebc5dfa

45133c5 doc: clarify `git range-diff` add/delete output (Lőrinc)

Pull request description:

  ### Problem
  Range diffs in git are useful after PR rebases, but it has an easy-to-misread failure mode: if it cannot match a commit between the old and new ranges, it will show the old commit as removed (<) and the new commit as added (>), without showing any patch contents for that commit.
  It can look like there were no code changes when in reality the commit was just treated as unrelated and needs full re-review.

  ### Example

  ```bash
  git fetch upstream ff338fd dd76338
  git range-diff ff338fd...dd76338
  ```

  This produced output like:
  ```patch
  1:  0ca4295 = 93:  139aa4b bench: add on-disk `HaveInputs` benchmark
  2:  4b32181 <  -:  ---------- test: add `HaveInputs` call-path unit tests
  -:  ---------- > 94:  277c57f test: add `HaveInputs` call-path unit tests
  3:  8c57687 ! 95:  c0c94ec dbwrapper: have `Read` and `Exists` reuse `ReadRaw`
  @@ Metadata
  ## Commit message ##
     dbwrapper: have `Read` and `Exists` reuse `ReadRaw`

  -    `ExistsImpl` was removed since it duplicates `CDBWrapper::ReadImpl` (except that it copies the resulting string on success, but that will be needed for caching anyway).
  +    `ExistsImpl` was removed since it duplicates `CDBWrapper::ReadImpl`.
  ```

  Even though the subject matches, there is no diff shown because the commits did not match - the reviewer could think that only the commit message was changed.
  This should be treated as **unmatched** rather than **unchanged**.
  If you expected a match, you can try increasing the search effort:
  ```bash
  git range-diff --creation-factor=95 ff338fd...dd76338
  ```
  which would show for example:
  ```patch
  1:  0ca4295 = 93:  139aa4b bench: add on-disk `HaveInputs` benchmark
  2:  4b32181 ! 94:  277c57f test: add `HaveInputs` call-path unit tests
  @@ Commit message

       The tests document that `HaveInputs()` consults the cache first and that a cache miss pulls from the backing view via `GetCoin()`.

  +    Co-authored-by: Novo <eunovo9@gmail.com>
  +
    ## src/test/coins_tests.cpp ##
   @@ src/test/coins_tests.cpp: BOOST_FIXTURE_TEST_CASE(ccoins_flush_behavior, FlushTest)
        }
    }

  -+BOOST_AUTO_TEST_CASE(ccoins_haveinputs_cache_miss_uses_base_getcoin)
  ++BOOST_AUTO_TEST_CASE(ccoins_cache_behavior)
  ```

  ### Fix
  This PR updates `doc/productivity.md` to raise awareness and document this pitfall and mentions `--creation-factor` as a knob to try when the output seems unexpectedly empty.

ACKs for top commit:
  maflcko:
    review ACK 45133c5 🏦
  Sjors:
    ACK 45133c5
  rkrux:
    crACK 45133c5
  sedited:
    ACK 45133c5

Tree-SHA512: 52dcf6db51425a3ac9789627f80233fb1e3437f7a351acf4a761504d9917837aa1ff8c964605a842ee099fae9842946784f7603f9bffa7051429b2f04b7900be

fa6af85 refactor: Use static_cast<decltype(...)> to suppress integer sanitizer warning (MarcoFalke)
fa69297 util: Fix UB in SetStdinEcho when ENOTTY (MarcoFalke)

Pull request description:

  The call to `tcgetattr` may fail with `ENOTTY`, leaving the struct possibly uninitialized (UB).

  Fix this UB by returning early when `isatty` fails, or when `tcgetattr` fails. (Same for Windows)

  This can be tested by a command that fails valgrind before the change and passes after:

  ```
  echo 'pipe' | valgrind --quiet ./bld-cmake/bin/bitcoin-cli -stdinrpcpass uptime

ACKs for top commit:
  achow101:
    ACK fa6af85
  l0rinc:
    lightly tested code review ACK fa6af85
  sedited:
    ACK fa6af85

Tree-SHA512: 76e2fbcb6c323b17736ee057dbd5e932b2e8cbb7d9fe4488c1dc7ab6ea928a3cde7e72ca0a63f8c8c78871ccb8b669263b712c0e1b530d88f2d45ea41f071201

Normally, when a symlinked test script is executed directly (e.g.,
`./bld-cmake/test/functional/wallet_disable.py`), Python's default
behavior is to resolve the symlink of the script itself, setting
`sys.path[0]` to the directory containing the physical source file.
Consequently, the test framework util.py is imported from the source
tree, and `Path(__file__).parents[3]` correctly resolves to the source
root.

However, `feature_framework_testshell.py` is unique because it manually
inserts `Path(__file__).parent` into `sys.path`. That refers to the
build tree and when importing the test framework util.py, the
`Path(__file__).parents[3]` will incorrectly point to the build
directory instead of the source root.

Use `.resolve()` to ensure the Valgrind suppressions file path is always
calculated relative to the physical source file, regardless of how the
framework was imported.

…pl()

c1361fc netif: fix compilation warning in QueryDefaultGatewayImpl() (MarcoFalke)

Pull request description:

  ```
  src/common/netif.cpp:137:51: error: comparison of integers of different signs: 'int64_t' (aka 'long') and 'unsigned long' [-Werror,-Wsign-compare]
    137 |         for (nlmsghdr* hdr = (nlmsghdr*)response; NLMSG_OK(hdr, recv_result); hdr = NLMSG_NEXT(hdr, recv_result)) {
        |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/include/netlink/netlink.h:220:31: note: expanded from macro 'NLMSG_OK'
    220 | #define NLMSG_OK(_hdr, _len)            NL_ITEM_OK(_hdr, _len, NLMSG_HDRLEN, _NLMSG_LEN)
        |                                         ^                ~~~~  ~~~~~~~~~~~~
  /usr/include/netlink/netlink.h:203:10: note: expanded from macro 'NL_ITEM_OK'
    203 |         ((_len) >= _hlen && _LEN_M(_ptr) >= _hlen && _LEN_M(_ptr) <= (_len))
        |           ~~~~  ^  ~~~~~
  1 error generated.
  ```

  Happens on FreeBSD 15.0, with the default compiler (Clang 19).

  On FreeBSD 14, `/usr/include/netlink/netlink.h` contains:
  ```
   #define NLMSG_HDRLEN                    ((int)sizeof(struct nlmsghdr))
  ```

  On FreeBSD 15, `/usr/include/netlink/netlink.h` contains:
  ```
   #define NLMSG_HDRLEN                    (sizeof(struct nlmsghdr))
  ```

ACKs for top commit:
  maflcko:
    lgtm ACK c1361fc
  hebasto:
    ACK c1361fc.

Tree-SHA512: f8f00e2106fdf91550ab388a65bb8408fcf8c4557da9614e2e1dd90e70fc010dff72bfabbbec4315335afdedb546f7b554f5c98133c5aa1d3077c5641d94b956

…runner switch

fa18be2 test: Fix typo (MarcoFalke)
fac9326 ci: Set TEST_RUNNER_PORT_MIN in test-each after cirrus runner switch (MarcoFalke)

Pull request description:

  This is needed after the recent switch to cirrus runners in the task in commit c8c9c1e.

  Otherwise, the CI will fail:

  ```
   node1 stderr Error: Unable to bind to 127.0.0.1:12321 on this computer. Bitcoin Core is probably already running.
  ```

  (https://github.com/bitcoin/bitcoin/actions/runs/22398358349/job/64837827234?pr=31723#step:9:2605)

  Also, include a random second commit, so that the CI task is run in this pull.

ACKs for top commit:
  l0rinc:
    ACK fa18be2
  willcl-ark:
    ACK fa18be2

Tree-SHA512: 6b63f645bf62d3e951ca155cddf3dc562b7ce675ccae4f9179e2202679685b5c147844eb350bd219b173fe2bb976376d0caa073d3e827a48c13aa015f4745b2c

…est shell

fab51e4 test: Move valgrind.supp to the other sanitizer_suppressions files (MarcoFalke)
fa9cf81 test: Add missing resolve() to valgrind.supp file (MarcoFalke)

Pull request description:

  (see commit message for details)

  Can be tested via:

  ```
  ./bld-cmake/test/functional/feature_framework_testshell.py --valgrind
  ```

  Before:

  ```
  bitcoind exited with status 1 during initialization. ==1735813== FATAL: can't open suppressions file "/b-c/b-c-2/bld-cmake/contrib/valgrind.supp"
  ```

  After: (passes)

  Also, move the suppression file to all the other suppression files.

ACKs for top commit:
  frankomosh:
    Re-ACK fab51e4

Tree-SHA512: d3e3e130c0e2292ca3ab9e80d2ebec6b4edc7914280ed90fb4af8a65cd1c9edd19064d86375a6787b864925fe0e47bab2321f89b9be8d1613a0aebf4ec2443fe

fa36ade ci: [refactor] Drop last use of pwsh (MarcoFalke)
fae31b1 ci: [refactor] Move github_import_vs_env to python script (MarcoFalke)

Pull request description:

  The use of pwsh was a bit confusing and inconsistent around the exit code. See also #32672

  I think it is fine to drop it and purely use Bash/Python.

  This also moves a bit of code from the github yaml to the python script.

ACKs for top commit:
  m3dwards:
    Looks good! re-ACK fa36ade
  hodlinator:
    re-ACK fa36ade

Tree-SHA512: 78edffc60c58c476b0acca5224150169d154b0b818114844a04295af9ba19b7cdf1fb2afb738f6cafd6172f0f477d546018ebf95061eb5bd8bbb35e065a129d4

…rfaceQueue()

Co-Authored-By: stickies-v <stickies-v@protonmail.com>

When unloading a wallet, there may be unexecuted callbacks in the
validation interface queue that can still execute after we have
completed all of the other wallet shutdown tasks. Instead of letting
these run in the background, once the notifications are disconnected,
wait for the queue to drain before continuing with wallet shutdown.

…d`` errors on early startup

bbc8f1e ipc mining: Prevent ``Assertion `m_node.chainman' failed`` errors on early startup (Ryan Ofsky)
a7cabf9 init refactor: Only initialize node.notifications one time (Ryan Ofsky)
c8e332c init refactor: Remove node.init accesss in AppInitInterfaces (Ryan Ofsky)

Pull request description:

  This fixes ``Assertion `m_node.chainman' failed`` errors first reported #33994 (comment) when IPC mining methods are called before ChainstateManager is loaded.

  The fix works by making the `Init.makeMining` method wait until chainstate data is loaded. It's probably the simplest possible fix but other alternatives like moving the wait to `Mining.createNewBlock` were discussed in the thread #34661 (comment) and could be implemented later without changes to clients.

ACKs for top commit:
  Sjors:
    utACK bbc8f1e
  ismaelsadeeq:
    ACK bbc8f1e
  achow101:
    ACK bbc8f1e

Tree-SHA512: 3e2e4e28ccff364b2303efd06ce337a229c28609076638500acb29559f716a15ad99409c6970ce9ad91776d53e3f9d959f1bbbd144ea9a4a2fb578ddbf2da267

fa48f8c test: Add missing timeout_factor to zmq socket (MarcoFalke)

Pull request description:

  Fixes #34189

  Otherwise, the test may intermittently fail on slow CI systems that have `--timeout-factor=` properly set.

  It can be tested by running `./bld-cmake/test/functional/interface_zmq.py --timeout-factor=10` with this diff:

  ```diff
  diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
  index c7be6ab..b14cf2aee6 100644
  --- a/src/validationinterface.cpp
  +++ b/src/validationinterface.cpp
  @@ -166,2 +166,3 @@ void ValidationSignals::SyncWithValidationInterfaceQueue()
               LOG_EVENT(fmt, local_name, __VA_ARGS__);           \
  +            UninterruptibleSleep(45ms); \
               event();                                           \
  diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py
  index 6717007..eee377daea 100755
  --- a/test/functional/interface_zmq.py
  +++ b/test/functional/interface_zmq.py
  @@ -176,3 +176,3 @@ class ZMQTest (BitcoinTestFramework):
           for sub in subscribers:
  -            sub.socket.set(zmq.RCVTIMEO, recv_timeout*1000)
  +            sub.socket.set(zmq.RCVTIMEO, int(recv_timeout * 1000))

  @@ -271,3 +271,3 @@ class ZMQTest (BitcoinTestFramework):
               [(topic, address) for topic in ["hashblock", "hashtx"]],
  -            recv_timeout=2)  # 2 second timeout to check end of notifications
  +            recv_timeout=0.2)  # 2 second timeout to check end of notifications
           self.disconnect_nodes(0, 1)
  ```

  Before this pull: Test fails with `zmq.error.Again: Resource temporarily unavailable`

  After this pull: Test passes

ACKs for top commit:
  l0rinc:
    code review ACK fa48f8c
  achow101:
    ACK fa48f8c

Tree-SHA512: 5ff8bdd807ff4b644daa634bb7b469da3da3915f58afba63a90e662df99cbebc86636e34e2b1b313c8629773aef2a239fb3025226a84d2ec22f6ecd4cea666c4

…e_broadcast.py

da7f70a test: use port 0 for I2P addresses in p2p_private_broadcast.py (Vasil Dimov)
a8ebcfd test: let connections happen in any order in p2p_private_broadcast.py (Vasil Dimov)
67696b2 net: extend log message to include attempted connection type (Vasil Dimov)

Pull request description:

  If the following two events happen:

  * (likely) the automatic 10 initial connections are not made to all
    networks
  * (unlikely) the network-specific logic kicks in almost immediately.
    It is using exponential distribution with a mean of 5 minutes
    (`rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)`).

  So if both happen, then the 11th connection may not be the expected
  private broadcast, but a network-specific connection.

  Fix this by retrieving the connection type from
  `destinations_factory()`. This is more flexible because it allows
  connections to happen in any order and does not break if e.g. the 11th
  connection is not the expected first private broadcast.

  This also makes the test run faster:
  before: 19-44 sec
  now: 10-25 sec
  because for example there is no need to wait for the initial 10
  automatic outbound connections to be made in order to proceed.

  Fixes: #34387

ACKs for top commit:
  achow101:
    ACK da7f70a
  andrewtoth:
    ACK da7f70a
  mzumsande:
    Code Review ACK da7f70a

Tree-SHA512: 7c293e59c15c148a438e0119343b05eb278205640658c99336d4caf4848c5bae92b48e15f325fa616cbc9d5f394649abfa02406a76e802cffbd3d312a22a6885

…connecting chain notifications

98e8af4 wallet: Drain validation interface queue after notifications disconnect (Ava Chow)
52992eb interfaces: Add waitForNotifications() to call SyncWithValidationInterfaceQueue() (Ava Chow)

Pull request description:

  When the wallet disconnects chain notifications, it is expecting no further notifications to execute, but this is not the case. This results in test failures such as in #34354. Instead of disconnecting the notifications and continuing shutdown, we should wait for the validation interface queue to be drained before the rest of wallet shutdown. This is achieved by adding an `interfaces::Chain::waitForNotifications()` function which calls `SyncWithValidationInterfaceQueue()`.

  Fixes #34354

ACKs for top commit:
  stickies-v:
    utACK 98e8af4
  furszy:
    ACK 98e8af4
  rkrux:
    crACK 98e8af4
  sedited:
    ACK 98e8af4

Tree-SHA512: 263628556f740cb633d3970c22a0dfdb52a644bd1d0cd5a69c2970524edbb0e25d592cb39fc9bf1d0c281eebce09578526e2958dffee9026fc7473db35bd0dec